Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

refactor(mcp): decompose OpenBuiltToolProvider (reduce PHPMD complexity, behavior-preserving)#153

Merged
rubenvdlinde merged 1 commit into
developmentfrom
refactor/toolprovider-complexity
May 26, 2026
Merged

refactor(mcp): decompose OpenBuiltToolProvider (reduce PHPMD complexity, behavior-preserving)#153
rubenvdlinde merged 1 commit into
developmentfrom
refactor/toolprovider-complexity

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What

Pure extract-and-delegate refactor of lib/Mcp/OpenBuiltToolProvider.php. No behavior change — every branch, return shape, exception, MCP tool name/schema, and side effect is preserved identically.

Why

OpenBuiltToolProvider was 1165 lines with PHPMD violations:

Metric Before After
Class length 1165 lines 319 lines
ExcessiveClassLength FAIL (>1000) PASS
ExcessiveClassComplexity 137 (threshold 50) PASS (pure dispatcher)
handleUpsertSchema NPath 14 256 PASS
handleUpsertPage NPath 6 336 PASS
handleAddWidget NPath 816 PASS
handleUpsertMenuItem NPath 3 168 PASS
handlePromoteVersion NPath 252 PASS
lib/Mcp/ PHPMD overall FAIL 0 violations

What was extracted

9 new files under lib/Mcp/Handler/:

  • AbstractToolHandler — shared base: errorResult, requireAuthenticatedUser, isValidSlug, buildDeepLink, sourceDescriptor, toArray, extractUuid, loadVersion, saveVersionManifest
  • ListAppsHandleropenbuilt.listApps
  • GetAppManifestHandleropenbuilt.getAppManifest
  • CreateAppHandleropenbuilt.createApp
  • PromoteVersionHandleropenbuilt.promoteVersion
  • UpsertSchemaHandleropenbuilt.upsertSchema (also extracts validateSlugsAndTitle + normaliseSchemaBody + findExistingSchema + attachSchemaToRegister)
  • UpsertPageHandleropenbuilt.upsertPage (extracts upsertPageInList)
  • AddWidgetHandleropenbuilt.addWidget (extracts findPageIndex + appendWidget)
  • UpsertMenuItemHandleropenbuilt.upsertMenuItem (extracts upsertMenuItemInList)

OpenBuiltToolProvider is now a thin dispatcher: constants → TOOL_DESCRIPTORSinvokeTool (string-map lookup) → makeHandler factory → handler .handle($args). The isAdmin public method stays in the provider (sole user of IGroupManager).

Verification

  • php -l — no syntax errors on all 10 files
  • vendor/bin/phpmd lib/Mcp text design,codesizeexit 0, 0 violations
  • vendor/bin/phpstan analyse lib --no-progressexit 0, no errors
  • vendor/bin/phpcs --standard=phpcs.xml lib/Mcp0 errors (pre-existing @spec warnings only, identical to original)
  • vendor/bin/phpunit tests/Unit — 192 tests, 111 pre-existing errors (all OCA\OpenRegister\Service\ObjectService not exist — same count before and after; no NC environment)

What remains inline / why

Nothing handler-logic lives in the provider. The @spec warning on public methods is pre-existing on the original file and requires a project-level spec ticket — out of scope for a complexity refactor.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant